Skip to content

feat(worker): add /v1alpha Review API on D1 - #426

Merged
richardthe3rd merged 9 commits into
mainfrom
feat/worker-ratings-api
Jun 13, 2026
Merged

feat(worker): add /v1alpha Review API on D1#426
richardthe3rd merged 9 commits into
mainfrom
feat/worker-ratings-api

Conversation

@richardthe3rd

@richardthe3rd richardthe3rd commented Jun 12, 2026

Copy link
Copy Markdown
Owner

First step towards an online "my festival". Clients submit a star rating and/or a "would recommend" answer and get back the shared aggregate. Conforms to the v1alpha proto contract merged in #425. Split out from #423.

Changes

Storage

  • Single D1-backed reviews table — one row per (bucket, festival, drink, device) with upsert semantics so re-reviewing never inflates counts
  • star_rating and recommend columns are independently nullable: a caller can rate without answering the recommendation question, or vice versa
  • Single migration 0001_create_reviews_table.sql
  • Every row and query scoped by bucket (test or prod, derived from request origin); RATINGS_BUCKET worker var can pin it

API (resource-oriented, conforms to v1alpha proto contract in proto/)

The Review is a singleton per (caller, drink). Caller identity comes from the X-Device-Id request header — the device ID never appears in resource names, so the sign-in upgrade (phase 3) is transparent to clients.

Method Path Purpose
PATCH /v1alpha/festivals/{f}/drinks/{d}/review Upsert review (starRating and/or wouldRecommend)
GET /v1alpha/festivals/{f}/drinks/{d}/review Get caller's review
DELETE /v1alpha/festivals/{f}/drinks/{d}/review Remove caller's review
GET /v1alpha/festivals/{f}/reviews List caller's reviews at a festival
GET /v1alpha/festivals/{f}/reviewSummaries/{d} Aggregate for one drink
GET /v1alpha/festivals/{f}/reviewSummaries Paginated list of aggregates (AIP-158)

PATCH body: { starRating?: 1–5, wouldRecommend?: bool, updateMask?: "starRating,wouldRecommend" }. Use updateMask to update one signal without clearing the other. Structured google.rpc.Status errors (AIP-193). CORS extended to GET/PATCH/DELETE with X-Device-Id allowed header. Unknown /v1alpha routes 404 instead of proxying upstream.

Implementation

  • reviews.ts + shared.ts — TypeScript, response bodies typed against the generated OpenAPI types in src/api-types.ts (proto → OpenAPI → openapi-typescript). A field rename in the proto surfaces as a compile error here.
  • tsconfig.json added; npm run typecheck (tsc --noEmit, strict mode) added to test:worker in mise
  • package.json: typecheck script; typescript and @cloudflare/workers-types added as devDependencies

Tests

  • 85 vitest tests: pure-helper unit tests + integration tests for upsert, partial-field update via updateMask, aggregation, validation, deletion, list, pagination, bucket isolation, and missing X-Device-Id header (simulated local D1, no real database needed)

CI

  • New proto job: runs buf lint on every proto-touching PR/push; runs buf breaking on PRs only (FILE stability, appropriate for v1alpha — switch to WIRE_JSON_COMPATIBLE when the API graduates to v1)

Deploy notes

One-time provisioning before first deploy:

cd cloudflare-worker
wrangler d1 create cbf-myfestival            # prints the database_id
# paste the id into wrangler.toml [[d1_databases]].database_id
wrangler d1 migrations apply cbf-myfestival  # applies migrations/*.sql

The deploy CLOUDFLARE_API_TOKEN must include D1: Edit in addition to Workers Scripts: Edit.

Regenerate types after proto changes:

MISE_ENV=dev ./bin/mise run proto:generate       # proto → openapi.yaml
MISE_ENV=dev ./bin/mise run proto:clients:types   # openapi.yaml → src/api-types.ts

claude added 3 commits June 12, 2026 20:48
First step towards an online "my festival". Clients submit a drink rating
and get back the shared aggregate (count + average + their own rating).

- New /v1/ratings endpoints on the existing proxy worker:
  POST/DELETE upsert/remove a device's rating, GET single + batch aggregates.
- D1-backed storage with upsert semantics (one row per device/drink) so
  re-rating never inflates counts. Anonymous device_id now; user_id column
  reserved for the sign-in upgrade.
- Every row/query scoped by a `bucket` so test traffic stays isolated from
  production data; bucket derived from origin, overridable via RATINGS_BUCKET.
- CORS extended to POST/DELETE.
- Full vitest coverage against a simulated local D1 (no real database needed):
  pure-helper unit tests plus integration tests for upsert, aggregation,
  validation, deletion and bucket isolation. 78 worker tests pass.

The wrangler.toml database_id is a placeholder; local dev and tests use a
simulated D1. README documents the endpoints and the one-time
`wrangler d1 create` / migrations-apply provisioning before first deploy.
A yes/no "would recommend" signal, separate from the star rating, so each
drink can surface a "% would recommend".

- New /v1/recommendations endpoints mirroring ratings (POST/DELETE upsert,
  GET single + batch). Aggregate reports total responses, "yes" count and
  the recommend percentage, plus the caller's own answer.
- Stored in a new `recommendations` table in the same D1 database, with the
  same per-device upsert and bucket-isolation model.
- Extracted shared bucket/id-validation/JSON/REST-routing plumbing into
  shared.js so ratings and recommendations stay thin; ratings refactored to
  consume it (behaviour unchanged).
- 19 new tests (pure helpers + integration for upsert, aggregation,
  validation, deletion, bucket isolation). 97 worker tests pass.

README documents the new endpoints; migration 0002 adds the table.
Rework the /v1 API to conform to the proto contract and Google's AIPs.

BREAKING CHANGE: replaces the flat POST/DELETE /v1/ratings endpoints with
resource-oriented routes. Nothing consumes them yet (no client, placeholder
DB), so this is a safe pre-launch change.

- Resource names: PATCH/GET/DELETE on
  /v1/festivals/{f}/drinks/{d}/ratings/{device} (and .../recommendations/...).
- Upsert via PATCH with allow_missing semantics (AIP-134); bodyless DELETE
  that is NOT_FOUND when absent (AIP-135).
- Read aggregates as RatingSummary / RecommendationSummary resources:
  GET .../{f}/ratingSummaries/{d} and a paginated list
  GET .../{f}/ratingSummaries (page_size/page_token/next_page_token +
  total_size, keyset cursor) (AIP-158).
- Structured google.rpc.Status errors with ErrorInfo reason+domain (AIP-193).
- RFC3339 update_time; camelCase resource fields matching the proto JSON
  mapping; dropped redundant your_* (client is local-first and knows its own).
- Generic family engine in shared.js drives both resources; CORS now allows
  GET/PATCH/DELETE; unknown /v1 routes 404 instead of proxying upstream.
- 87 worker tests pass.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the Cloudflare proxy worker with a new resource-oriented /v1 API backed by D1 to collect per-device drink ratings and “would recommend” answers and return aggregated summaries, as a first step toward an online “my festival”.

Changes:

  • Adds D1 schema + worker routing for ratings / recommendations record endpoints and their aggregate summary/list endpoints.
  • Introduces a shared “resource family” engine (shared.js) used by thin ratings.js and recommendations.js handlers.
  • Adds vitest + Miniflare/D1 migration setup and integration tests covering upsert/read/delete, summaries, pagination, CORS, and bucket isolation.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
cloudflare-worker/wrangler.toml Adds D1 binding/config for ratings storage (placeholder database_id) and migrations dir.
cloudflare-worker/worker.js Routes /v1 requests to ratings/recommendations handlers; returns 404 for unknown /v1 routes; expands CORS methods.
cloudflare-worker/vitest.config.js Loads D1 migrations at config time and exposes them to tests via bindings + setup file.
cloudflare-worker/shared.js Implements shared routing/validation, structured errors, pagination, and D1 queries for resource families.
cloudflare-worker/ratings.js Defines ratings family (1–5 integer validation) and serialization/summary logic.
cloudflare-worker/recommendations.js Defines recommendations family (boolean validation) and serialization/summary logic.
cloudflare-worker/migrations/0001_create_ratings_table.sql Creates ratings table with bucket scoping and aggregate index.
cloudflare-worker/migrations/0002_create_recommendations_table.sql Creates recommendations table with bucket scoping and aggregate index.
cloudflare-worker/README.md Documents the new /v1 API and D1 provisioning steps.
cloudflare-worker/test/apply-migrations.js Applies migrations to simulated D1 before tests run.
cloudflare-worker/test/ratings.test.js Integration + helper tests for ratings endpoints, pagination, and bucket isolation.
cloudflare-worker/test/recommendations.test.js Integration + helper tests for recommendations endpoints and bucket isolation.
cloudflare-worker/test/cors.test.js Updates preflight assertions for expanded allowed methods.

Comment thread cloudflare-worker/shared.js Outdated
Comment on lines +135 to +138
const segments = parseV1Path(url.pathname);
if (!segments || segments[0] !== "festivals" || segments.length < 3) {
return null;
}
Comment thread cloudflare-worker/shared.js Outdated
Comment on lines +26 to +31
export function resolveBucket(origin, env) {
if (env && typeof env.RATINGS_BUCKET === "string" && env.RATINGS_BUCKET) {
return env.RATINGS_BUCKET;
}
return isProductionOrigin(origin) ? "prod" : "test";
}
Comment thread cloudflare-worker/shared.js Outdated
Comment on lines +252 to +266
async function getRecord(ctx) {
const { family, festivalId, drinkId, deviceId, corsHeaders } = ctx;
const row = await readRow(ctx);
if (!row) {
return errorResponse(
404,
"NOT_FOUND",
"No such rating",
"NOT_FOUND",
corsHeaders,
);
}
const name = writeResourceName(family, festivalId, drinkId, deviceId);
return jsonResponse(family.serializeResource(name, row), 200, corsHeaders);
}
Comment thread cloudflare-worker/shared.js Outdated
Comment on lines +313 to +336
async function deleteRecord(ctx) {
const { db, family, bucket, festivalId, drinkId, deviceId, corsHeaders } =
ctx;
const result = await db
.prepare(
`DELETE FROM ${family.table} ` +
"WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?",
)
.bind(bucket, festivalId, drinkId, deviceId)
.run();

// AIP-135: deleting a missing resource is NOT_FOUND.
const changes = result.meta ? result.meta.changes : 0;
if (!changes) {
return errorResponse(
404,
"NOT_FOUND",
"No such rating",
"NOT_FOUND",
ctx.corsHeaders,
);
}
return jsonResponse({}, 200, corsHeaders);
}
Comment thread cloudflare-worker/README.md Outdated
Comment on lines +150 to +151
The deploy `CLOUDFLARE_API_TOKEN` must include **D1: Edit** in addition to
Workers Scripts: Edit. To wipe test data: `DELETE FROM ratings WHERE bucket='test'`.
Comment on lines +3 to +5
// Apply the ratings schema to the per-test simulated D1 before any test runs.
// `TEST_MIGRATIONS` is provided by vitest.config.js via readD1Migrations().
await applyD1Migrations(env.RATINGS_DB, env.TEST_MIGRATIONS);
claude added 4 commits June 13, 2026 09:02
Rebases the ratings/recommendations worker from PR #426 onto current
main and updates the implementation to conform to the v1alpha proto
contract merged in PR #425.

Changes from the original design:
- URL prefix: /v1/ → /v1alpha/
- Separate `ratings/{device}` + `recommendations/{device}` collections
  replaced by a single `Review` singleton at `drinks/{d}/review`
- Device ID moves from the URL to the `X-Device-Id` request header;
  it no longer appears in resource names (auth-upgrade transparent)
- Separate `ratingSummaries` + `recommendationSummaries` merged into
  `reviewSummaries` (combined ratingCount + responseCount/recommendRate)
- `starRating` + `wouldRecommend` signals independently nullable;
  `updateMask` in the PATCH body allows updating one without clearing the other
- DB schema: two tables → single `reviews` table with nullable columns

New routes:
  GET/PATCH/DELETE /v1alpha/festivals/{f}/drinks/{d}/review
  GET              /v1alpha/festivals/{f}/reviews
  GET              /v1alpha/festivals/{f}/reviewSummaries[/{d}]

Implementation:
- reviews.js replaces ratings.js + recommendations.js
- shared.js retains utility functions (bucket, errors, pagination)
- Single migration: 0001_create_reviews_table.sql
- 85 vitest tests pass (pure helpers, upsert, get/delete, list,
  summaries, bucket isolation, missing header, routing)
- CORS: allow X-Device-Id header alongside Content-Type

https://claude.ai/code/session_01VVTCRjdHqcTJCVEjEVQV6C
Populated by mise during toolchain install (buf 1.70.0 via aqua backend).

https://claude.ai/code/session_01VVTCRjdHqcTJCVEjEVQV6C
Convert reviews.js → reviews.ts and shared.js → shared.ts. Response
bodies (Review, ReviewSummary, ListReviewsResponse, etc.) are now typed
against the generated src/api-types.ts (proto → OpenAPI → openapi-typescript),
so a field rename or type change in the proto surfaces as a compile error
in the implementation.

- types: Review, ReviewSummary, ListReviewsResponse, ListReviewSummariesResponse
  imported from components["schemas"][...] in the generated api-types.ts
- Env interface (RATINGS_DB: D1Database, RATINGS_BUCKET?) centralised in shared.ts
- D1 row shapes (ReviewRow, SummaryRow, etc.) typed for all queries
- tsc --noEmit passes clean (strict mode, moduleResolution: bundler)
- 85 vitest tests still pass
- package.json: add typecheck script; tsconfig.json added
- mise.toml: test:worker now runs tsc before vitest

Regenerate types after proto changes:
  MISE_ENV=dev ./bin/mise run proto:generate
  MISE_ENV=dev ./bin/mise run proto:clients:types

https://claude.ai/code/session_01VVTCRjdHqcTJCVEjEVQV6C
Add a proto job to CI that runs buf lint and buf breaking on every PR
that touches proto/. Breaking change detection uses FILE stability level
(configured in proto/buf.yaml), appropriate for v1alpha APIs — catches
source-breaking changes to generated code while allowing additive changes.

buf breaking only runs on pull_request events (bufbuild/buf-action skips
it on push to main where the PR is already merged). Lint runs on both.

Switch breaking.use from FILE to WIRE_JSON_COMPATIBLE in proto/buf.yaml
when the API graduates from v1alpha to v1 stable.

https://claude.ai/code/session_01VVTCRjdHqcTJCVEjEVQV6C
@richardthe3rd richardthe3rd changed the title feat(worker): add aggregate ratings and recommendations API on D1 feat(worker): add /v1alpha Review API on D1 Jun 13, 2026
@codecov

codecov Bot commented Jun 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

claude added 2 commits June 13, 2026 09:36
- Validate drinkId before getReviewSummary to prevent an injection
  path through the resource-name segments (INVALID_RESOURCE_NAME 400)
- Reject unknown updateMask fields rather than silently ignoring them
  (UNKNOWN_FIELD_MASK 400), matching AIP-134 contract guarantees
- Eliminate post-write readRow() in upsert: compute finalStarRating /
  finalRecommend before writing and build the response from those values,
  removing one DB round trip and closing a TOCTOU race where a concurrent
  DELETE between write and re-read caused a non-null assertion crash

Test: adds UNKNOWN_FIELD_MASK case; all 86 tests pass

https://claude.ai/code/session_01VVTCRjdHqcTJCVEjEVQV6C
TypeScript 6 breaks npm ci: openapi-typescript@7.13.0 requires
peer typescript@"^5.x". Downgrade to ^5.9.3 to restore compatibility.

Also apply prettier formatting to reviews.ts, shared.ts, and
reviews.test.js which CI's fmt check was rejecting.

https://claude.ai/code/session_01VVTCRjdHqcTJCVEjEVQV6C
@richardthe3rd
richardthe3rd force-pushed the feat/worker-ratings-api branch from 8c04fd0 to 1f3e4bd Compare June 13, 2026 09:56
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Cloudflare Pages Preview

Your preview deployment is ready!

Preview URL: https://feat-worker-ratings-api.staging-cambeerfestival.pages.dev

This preview will be automatically updated when you push new commits to this PR.

@richardthe3rd
richardthe3rd merged commit 9dfc334 into main Jun 13, 2026
20 checks passed
@github-actions github-actions Bot mentioned this pull request Jun 13, 2026
@richardthe3rd
richardthe3rd deleted the feat/worker-ratings-api branch June 13, 2026 10:18
richardthe3rd pushed a commit that referenced this pull request Jun 13, 2026
Adds a Redoc-rendered API docs page at /api-docs/ in the Cloudflare Pages
deployment. The OpenAPI spec is generated from proto at build time (not
committed), consistent with the proto-in-CI pattern from #426.

- web/api-docs/index.html: Redoc page loading openapi.yaml from jsDelivr CDN
- web/api-docs/.gitignore: openapi.yaml is generated, not committed
- web/_headers: /api-docs/* CSP override allowing cdn.jsdelivr.net for Redoc
- ci.yml build-web: buf generate + copy openapi.yaml before flutter build
- mise.dev.toml proto:generate: also copies to web/api-docs/ for local dev

https://claude.ai/code/session_015uTnGiC56cEELZMH2cQQU4
richardthe3rd added a commit that referenced this pull request Jun 13, 2026
)

* docs(api): publish MyFestival OpenAPI spec via Redoc at /api-docs/

Adds a Redoc-rendered API docs page at /api-docs/ in the Cloudflare Pages
deployment. The OpenAPI spec is generated from proto at build time (not
committed), consistent with the proto-in-CI pattern from #426.

- web/api-docs/index.html: Redoc page loading openapi.yaml from jsDelivr CDN
- web/api-docs/.gitignore: openapi.yaml is generated, not committed
- web/_headers: /api-docs/* CSP override allowing cdn.jsdelivr.net for Redoc
- ci.yml build-web: buf generate + copy openapi.yaml before flutter build
- mise.dev.toml proto:generate: also copies to web/api-docs/ for local dev

https://claude.ai/code/session_015uTnGiC56cEELZMH2cQQU4

* docs: add API docs link to README

https://claude.ai/code/session_015uTnGiC56cEELZMH2cQQU4

* fix(api-docs): exempt /api-docs/ from Flutter SPA catch-all rewrite

Cloudflare Pages does not resolve implicit directory indexes before
evaluating _redirects rewrite rules, so /api-docs/ was being caught by
/* /index.html 200 before the Redoc page could be served.

Adding explicit /api-docs and /api-docs/ rules before the catch-all
routes those two paths to /api-docs/index.html directly.

https://claude.ai/code/session_015uTnGiC56cEELZMH2cQQU4

* fix(api-docs): serve Redoc from self rather than CDN to avoid CSP issues

Downloading redoc.standalone.js at build time (CI) and local dev
(proto:generate) so it is served from 'self', which the existing
Flutter CSP already allows. Removes the /api-docs/* CSP override
that was added to permit cdn.jsdelivr.net.

https://claude.ai/code/session_015uTnGiC56cEELZMH2cQQU4

---------

Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot mentioned this pull request Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants